Skip to content
This repository has been archived by the owner on May 11, 2022. It is now read-only.

issue #34 and #35 - adding new command to pull a #140

Merged

Conversation

bfitzpat
Copy link
Collaborator

@bfitzpat bfitzpat commented Jul 6, 2020

file from a URL, rename it, put it into a folder, and even unzip it

Signed-off-by: bfitzpat@redhat.com bfitzpat@redhat.com

@bfitzpat
Copy link
Collaborator Author

bfitzpat commented Jul 6, 2020

Issues #134 and #135

The question came up... is there an easy way to copy a file from a URL and copy it into a project?

There are a few possibilities in the VS Code Extension Marketplace, but nothing that worked quite how we needed it to. As a result, we decided to roll our own.

So let's say we have this image:

Spongebob Squarepants Finger Guns

We can find it at this URL: https://media.giphy.com/media/7DzlajZNY5D0I/giphy.gif

We've now created a new command that will take that URL and some additional optional parameters to do some cool things...

Copying a File into the Workspace root with no name change

Let's say we want to copy it into our project. I've created a new command that takes the URL, the file name, and an optional path for the workspace.

  • Click here to let it do its thing and download giphy.gif.
  • Click here to open giphy.gif

Here's the Didact URL: didact://?commandId=vscode.didact.copyFileURLtoWorkspaceCommand&text=https://media.giphy.com/media/7DzlajZNY5D0I/giphy.gif

Copying a File and Changing the Name

  • Click here to let it do its thing, but this time we'll change the filename.
  • Click here to open spongebob.gif

Here's the Didact URL: didact://?commandId=vscode.didact.copyFileURLtoWorkspaceCommand&text=https://media.giphy.com/media/7DzlajZNY5D0I/giphy.gif$$spongebob.gif

What if we want to change the name and put it into a specific folder?

This time we'll try a different Spongebob... we're getting excited!

Spongebob Squarepants Excited

  • Click here to let it do its thing and open the downloaded file, but this time we'll put it in a new folder and change the filename again.
  • Click here to open GIFs/spongebob.gif

Here's the Didact URL: didact://?commandId=vscode.didact.copyFileURLtoWorkspaceCommand&text=https://media.giphy.com/media/nDSlfqf0gn5g4/giphy.gif$$spongebob-excited.gif$$projectFilePath=GIFs

That's all well and good, but what if I have an archive file I want to pull down and unzip at the same time?

This is another issue (#135) that came up and now we can do that too! Let's take the Apache Camel K examples archive and unzip it into a new folder called camel-k-examples.

  • Click here to download and unzip the Apache Camel K examples archive into a new directory.
  • Click here to open camel-k-examples/README.md

Here's the Didact URL: didact://?commandId=vscode.didact.copyFileURLtoWorkspaceCommand&text=https://github.com/apache/camel-k/releases/download/1.0.1/camel-k-examples-1.0.1.tar.gz$$camel-k-examples.tar.gz$$projectFilePath=camel-k-examples$$true

Next Steps

We still have some testing to do and some validation steps to add around file overwriting and the like, but we're making some progress!

@bfitzpat
Copy link
Collaborator Author

bfitzpat commented Jul 6, 2020

Still need to see about testing (though not exactly sure how) and adding more validation around overwriting files, but I think this is a good start. I re-used code from the Camel K extension to handle the download and extract cases.

Copy link
Member

@apupier apupier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • what about adding the good examples that you put in comment of the PR directly as an example in the codebase? I think that it will really useful.
  • In example, the last parameter to extract or not is passed as a simple boolean. What do you think having to specify a parameter name such as "extract=true"? having just the $$true I was wondering what the true is for. It would also allows to add more parameters in the future without collision.
  • for testing, a first step would be to call the command through the didact entry point using the Didact url. (command.handler.processInputs ?)

src/extensionFunctions.ts Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@bfitzpat
Copy link
Collaborator Author

bfitzpat commented Jul 7, 2020

  • what about adding the good examples that you put in comment of the PR directly as an example in the codebase? I think that it will really useful.

Yes, I can add a new examples markdown that re-uses the examples I documented here.

  • In example, the last parameter to extract or not is passed as a simple boolean. What do you think having to specify a parameter name such as "extract=true"? having just the $$true I was wondering what the true is for. It would also allows to add more parameters in the future without collision.

Though I see what you're after there, that's definitely a much larger change than this PR. Can we spin that off into another JIRA to possibly handle in the future?

  • for testing, a first step would be to call the command through the didact entry point using the Didact url. (command.handler.processInputs ?)

Hmmm. Not sure I understand quite what you're suggesting here?

@apupier
Copy link
Member

apupier commented Jul 7, 2020

  • In example, the last parameter to extract or not is passed as a simple boolean. What do you think having to specify a parameter name such as "extract=true"? having just the $$true I was wondering what the true is for. It would also allows to add more parameters in the future without collision.

Though I see what you're after there, that's definitely a much larger change than this PR. Can we spin that off into another JIRA to possibly handle in the future?

Why is it more complicated than the other parameters? such as projectFilePath=camel-k-examples

@apupier
Copy link
Member

apupier commented Jul 7, 2020

  • for testing, a first step would be to call the command through the didact entry point using the Didact url. (command.handler.processInputs ?)

Hmmm. Not sure I understand quite what you're suggesting here?

in #140 (comment) you said We still have some testing to do

so I'm suggesting a way to test the functionality which is added in this PR. it doesn't sound a big chunk of work. providing the test when writing the feature is usually better.

@bfitzpat
Copy link
Collaborator Author

bfitzpat commented Jul 7, 2020

  • for testing, a first step would be to call the command through the didact entry point using the Didact url. (command.handler.processInputs ?)

Hmmm. Not sure I understand quite what you're suggesting here?

in #140 (comment) you said We still have some testing to do

so I'm suggesting a way to test the functionality which is added in this PR. it doesn't sound a big chunk of work. providing the test when writing the feature is usually better.

No, I get that we need tests. I'm more asking about your suggestion to call the command through the didact entry point using the Didact url. (command.handler.processInputs? )

What are you proposing as far as the test? (That's what I was getting at, sorry I wasn't clearer.)

@bfitzpat
Copy link
Collaborator Author

bfitzpat commented Jul 7, 2020

Why is it more complicated than the other parameters? such as projectFilePath=camel-k-examples

I am pondering getting rid of the projectFilePath= portion here after some thought, simply because it's implied that we're copying files into the user workspace at this level. That will make it more like the other commands where it's the order of the parameters that's important. Hopefully that will clarify this a little.

That said, I think adding an option for name/value pairs to be specified as part of the Didact url is an interesting one. At this point it's only special cases that use that approach and the rest of the parameters are by order. If you create a JIRA we can discuss this further in that context and I can point to examples more easily. :)

bfitzpat added 5 commits July 8, 2020 09:21
file from a URL, rename it, put it into a folder, and even unzip it

Signed-off-by: bfitzpat@redhat.com <bfitzpat@redhat.com>
Signed-off-by: bfitzpat@redhat.com <bfitzpat@redhat.com>
Signed-off-by: bfitzpat@redhat.com <bfitzpat@redhat.com>
Signed-off-by: bfitzpat@redhat.com <bfitzpat@redhat.com>
Signed-off-by: bfitzpat@redhat.com <bfitzpat@redhat.com>
@bfitzpat bfitzpat force-pushed the issue134-file-from-url-and-unzip branch from 95387d9 to a0b1510 Compare July 8, 2020 15:26
@bfitzpat
Copy link
Collaborator Author

bfitzpat commented Jul 8, 2020

Only question remaining is how much validation to add... Do we check to see if the file is there? If so, do we prompt the user before we overwrite it or reject it because it already exists?

Signed-off-by: bfitzpat@redhat.com <bfitzpat@redhat.com>
@bfitzpat
Copy link
Collaborator Author

bfitzpat commented Jul 8, 2020

didact-copy-file-confirm-overwrite

@bfitzpat
Copy link
Collaborator Author

bfitzpat commented Jul 8, 2020

didact-copy-file-confirm-unzip-overwrite

Signed-off-by: bfitzpat@redhat.com <bfitzpat@redhat.com>
@bfitzpat bfitzpat requested a review from apupier July 8, 2020 16:59
Signed-off-by: bfitzpat@redhat.com <bfitzpat@redhat.com>
apupier
apupier previously requested changes Jul 9, 2020
Copy link
Member

@apupier apupier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • when answering "No" for overwriting, it is overwriting:

ezgif com-optimize

examples/copyFileURL.example.didact.md Outdated Show resolved Hide resolved
Signed-off-by: bfitzpat@redhat.com <bfitzpat@redhat.com>
@lhein lhein dismissed apupier’s stale review July 13, 2020 14:33

requested change has been implemented

@bfitzpat bfitzpat merged commit 4e65137 into redhat-developer:master Jul 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants